Skip to content

reduce spread code size by passing false to get_spread_update for static attributes #4896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

reduce spread code size by passing false to get_spread_update for static attributes #4896

wants to merge 3 commits into from

Conversation

tanhauhau
Copy link
Member

@tanhauhau tanhauhau commented May 24, 2020 •
edited
Loading

Reduce code size by:

  1. use false for get_spread_update for static attributes to reduce codesize
  set_attributes(div, get_spread_update(div_levels, [
-   { a: "1" },
+   false,
-   { b: "2" },
+   false,
    dirty & /*name*/ 1 && { name: /*name*/ ctx[0] },
-   { a: 1 }
+   false
  ]));
  1. added get_attributes_for_spread util to reuse for initialise attribute data for spread
  2. use dynamic dependencies instead of dependencies for spreading attribute, reduce unnecessary code, if the dependencies are not dynamic.

Align the code for spreading in Component and element,

  1. apply what Fix non-object value of spread attributes variable #3306 did for Component on element, add check on spreading for element as well.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR relates to an outstanding issue, so please reference it in your PR, or create an explanatory one for discussion. In many cases features are absent for a reason.
  • This message body should clearly illustrate what problems it solves. If there are related issues, remember to reference them.
  • Ideally, include a test that fails without this PR but passes with it. PRs will only be merged once they pass CI. (Remember to npm run lint!)

Tests

  • Run the tests tests with npm test or yarn test)

@Conduitry
Copy link
Member

Does this also fix a bug? I'm unclear on what the new test does.

Unrelatedly, this also now conflicts with master.

@tanhauhau tanhauhau force-pushed the tanhauhau/optimise-spread-code branch from fe77890 to f3b0809 Compare May 30, 2020 01:08
@tanhauhau tanhauhau force-pushed the tanhauhau/optimise-spread-code branch from f3b0809 to 9f0b678 Compare May 30, 2020 01:26
@tanhauhau
Copy link
Member Author

@Conduitry the PR is mainly to reduce code size.

the extra test case was copied from the one we had for Components https://github.com/sveltejs/svelte/tree/master/test/runtime/samples/spread-component-dynamic-non-object

@tanhauhau
Copy link
Member Author

tanhauhau commented May 30, 2020 •
edited
Loading

Did some benchmark, based on @kevmodrome @halfnelson suggestion,

tried to compile + bundle + minify components in svelte-material-ui, generally if contain 1 ~ 2 spread attribute, the size actually increase by ~1%. I tried compile the demo page, which 1 page include multiple different components, the bundle size dropped to 1 ~ 2%.

here are some results:

benchmark results
in [packages/](https://github.com/hperrin/svelte-material-ui/tree/master/packages)
component baseline with changes % change
button/Button.svelte 12.9 kB 12.9 kB 100.02%
button/Group.svelte 6.76 kB 6.86 kB 101.54%
card/Actions.svelte 6.88 kB 6.98 kB 101.51%
card/Card.svelte 6.88 kB 6.99 kB 101.51%
card/Media.svelte 6.87 kB 6.98 kB 101.51%
card/PrimaryAction.svelte 8.6 kB 8.71 kB 101.22%
checkbox/Checkbox.svelte 10.2 kB 10.3 kB 100.61%
chips/Checkmark.svelte 5.06 kB 5.17 kB 102.03%
chips/Chip.svelte 7.7 kB 7.81 kB 101.35%
chips/Set.svelte 10.2 kB 10.3 kB 101.04%
common/A.svelte 6.48 kB 6.59 kB 101.6%
common/Aside.svelte 6.37 kB 6.47 kB 101.54%
common/Button.svelte 6.37 kB 6.47 kB 101.54%
common/ClassAdder.svelte 6.26 kB 6.32 kB 101.04%
common/Div.svelte 6.37 kB 6.46 kB 101.54%
common/Footer.svelte 6.37 kB 6.47 kB 101.54%
common/H1.svelte 6.37 kB 6.46 kB 101.54%
common/H2.svelte 6.37 kB 6.46 kB 101.54%
common/H3.svelte 6.37 kB 6.46 kB 101.54%
common/H4.svelte 6.37 kB 6.46 kB 101.54%
common/H5.svelte 6.37 kB 6.46 kB 101.54%
common/H6.svelte 6.37 kB 6.46 kB 101.54%
common/Header.svelte 6.37 kB 6.47 kB 101.54%
common/Icon.svelte 7.97 kB 8.05 kB 101.05%
common/Img.svelte 5.64 kB 5.74 kB 101.84%
common/Label.svelte 7.3 kB 7.34 kB 100.56%
common/Li.svelte 6.37 kB 6.46 kB 101.54%
common/Section.svelte 6.38 kB 6.47 kB 101.54%
common/Span.svelte 6.37 kB 6.47 kB 101.54%
common/Ul.svelte 6.37 kB 6.46 kB 101.54%
data-table/Body.svelte 6.61 kB 6.71 kB 101.57%
data-table/Cell.svelte 8.45 kB 8.55 kB 101.22%
data-table/DataTable.svelte 8.95 kB 9.04 kB 100.97%
data-table/Head.svelte 6.45 kB 6.54 kB 101.52%
data-table/Row.svelte 7.33 kB 7.45 kB 101.61%
dialog/Dialog.svelte 8.58 kB 8.64 kB 100.78%
drawer/Drawer.svelte 7.87 kB 7.98 kB 101.32%
fab/Fab.svelte 8.9 kB 9 kB 101.18%
floating-label/FloatingLabel.svelte 8.59 kB 8.68 kB 101.06%
form-field/FormField.svelte 8.19 kB 8.27 kB 101.06%
icon-button/IconButton.svelte 12.3 kB 12.4 kB 100.36%
image-list/ImageList.svelte 7.06 kB 7.16 kB 101.47%
line-ripple/LineRipple.svelte 6.46 kB 6.56 kB 101.61%
linear-progress/LinearProgress.svelte 8.08 kB 8.17 kB 101.08%
list/Item.svelte 14.2 kB 14.3 kB 100.51%
list/Label.svelte 6.68 kB 6.74 kB 100.94%
list/List.svelte 11.2 kB 11.3 kB 100.86%
list/Separator.svelte 7.21 kB 7.29 kB 101.04%
menu/Menu.svelte 14.3 kB 14.3 kB 100.42%
menu/SelectionGroup.svelte 7.11 kB 7.18 kB 101.11%
menu-surface/MenuSurface.svelte 10 kB 10.1 kB 101.05%
notched-outline/NotchedOutline.svelte 8.3 kB 8.41 kB 101.25%
paper/Paper.svelte 7.37 kB 7.48 kB 101.42%
radio/Radio.svelte 8.64 kB 8.7 kB 100.74%
select/Option.svelte 16.6 kB 16.6 kB 99.86%
select/Select.svelte 40.6 kB 40.2 kB 99.13%
slider/Slider.svelte 9.98 kB 10.1 kB 100.97%
snackbar/Snackbar.svelte 9.23 kB 9.32 kB 100.94%
switch/Switch.svelte 9.12 kB 9.16 kB 100.53%
tab/Tab.svelte 15.7 kB 15.6 kB 99.41%
tab-bar/TabBar.svelte 13.8 kB 13.7 kB 99.61%
tab-indicator/TabIndicator.svelte 8.97 kB 9.05 kB 100.97%
tab-scroller/TabScroller.svelte 8.85 kB 8.9 kB 100.6%
textfield/Input.svelte 6.91 kB 7.03 kB 101.75%
textfield/Textarea.svelte 6.43 kB 6.53 kB 101.62%
textfield/Textfield.svelte 29.8 kB 29.5 kB 99.07%
top-app-bar/Section.svelte 7.27 kB 7.39 kB 101.62%
top-app-bar/TopAppBar.svelte 8.02 kB 8.12 kB 101.31%
in [site/src/routes/demo](https://github.com/hperrin/svelte-material-ui/tree/master/site/src/routes/demo)
component baseline with changes % change
button.svelte 99.6 kB 99 kB 99.45%
card.svelte 85.7 kB 85.1 kB 99.29%
checkbox.svelte 27.6 kB 27.5 kB 99.5%
chips.svelte 39.1 kB 38.9 kB 99.43%
data-table.svelte 36.1 kB 36 kB 99.66%
dialog.svelte 75.5 kB 74.9 kB 99.11%
drawer.svelte 68.9 kB 68.4 kB 99.21%
elevation.svelte 17.3 kB 17.3 kB 100.02%
fab.svelte 35.9 kB 35.7 kB 99.54%
form-field.svelte 20.5 kB 20.5 kB 99.63%
icon-button.svelte 30.5 kB 30.3 kB 99.21%
image-list.svelte 25.1 kB 25 kB 99.52%
linear-progress.svelte 27 kB 26.8 kB 99.31%
list.svelte 83.4 kB 83 kB 99.46%
menu-surface.svelte 75.5 kB 74.7 kB 98.92%
menu.svelte 67.6 kB 67.1 kB 99.34%
paper.svelte 36.7 kB 36.5 kB 99.35%
radio.svelte 14.4 kB 14.4 kB 100.05%
ripple.svelte 6.83 kB 6.83 kB 100%
select.svelte 125 kB 124 kB 99.55%
slider.svelte 16.6 kB 16.7 kB 100.27%
snackbars.svelte 70 kB 69.2 kB 98.86%
switch.svelte 17.7 kB 17.7 kB 99.96%
tabs.svelte 47.7 kB 47.2 kB 99%
textfield.svelte 124 kB 123 kB 99.63%
theme.svelte 30.6 kB 30.5 kB 99.44%
top-app-bar-iframe.svelte 27.7 kB 27.6 kB 99.49%
top-app-bar.svelte 43 kB 42.7 kB 99.33%
typography.svelte 6.68 kB 6.68 kB 100%

not sure whether it is conclusively better? 🤷‍♂️

@benmccann
Copy link
Member

benmccann commented May 31, 2020 •
edited
Loading

For network latency, it'd probably be a good idea to have the benchmark minify and gzip the output (if it's not already) since that's how these files will normally be served especially for sites that would be performance-sensitive. Though once it's on the client device it will be necessary to parse and execute the uncompressed files. I'm not sure what the tradeoff is here in the case that the gzipped output get bigger/smaller and the uncompressed does the opposite as to which is more important

@stale
Copy link

stale bot commented Jun 26, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Jun 26, 2021
@stale stale bot removed the stale-bot label Jun 26, 2021
@stale stale bot removed the stale-bot label Jun 27, 2021
@dummdidumm
Copy link
Member

Given that the code reduction is small and unclear to be better in all cases, and given that the code is outdated, I'm closing this.

@dummdidumm dummdidumm closed this Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tanhauhau @Conduitry @benmccann @dummdidumm @pngwn